feat(sdk): add comprehensive DPoP (RFC 9449) support (DSPX-3397)#374
feat(sdk): add comprehensive DPoP (RFC 9449) support (DSPX-3397)#374dmihalcik-virtru wants to merge 24 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces support for custom DPoP keys in the SDKBuilder and implements DPoP nonce caching in TokenSource and AuthInterceptor to handle server-issued nonces. It also adds a new "supports" subcommand to the CLI to check for feature support (e.g., "dpop"). Regarding the feedback, the "supports" subcommand should avoid calling System.exit() directly, as this abruptly terminates the JVM and hinders testing; instead, it should implement Callable and return the exit code.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
5242c69 to
00414f1
Compare
X-Test Failure Report |
X-Test Results✅ java@pull-374-main |
- Add server-issued nonce caching infrastructure in TokenSource with per-origin storage - Add dpopKey() method to SDKBuilder for caller-supplied RSA keys (defaults to auto-generated ephemeral key) - Update AuthInterceptor to cache DPoP-Nonce from successful responses - Add 'supports dpop' CLI command for xtest feature detection - Extend TokenSource.getAuthHeaders() to accept optional nonce parameter for proof generation Implementation uses Nimbus OAuth2 SDK's DefaultDPoPProofFactory for RFC 9449 compliant DPoP proof generation with htm/htu/iat/jti claims (plus ath for resource endpoints). Current implementation uses RSA-2048/RS256 for DPoP keys. The SDK already had DPoP proof generation via Nimbus OAuth2 SDK; this PR adds nonce support infrastructure and makes the DPoP key configurable. Note: Full 401 retry logic with nonce challenges requires Connect RPC interceptor changes and is deferred to future work. Nonce caching infrastructure is in place for when retry logic is added. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
- AuthInterceptor.kt: fix resp.code→resp.status and resp.message.request() compilation errors; use ThreadLocal<URL> to thread request URL into responseFunction for nonce caching; change private→internal so SDKBuilder.java can access dpopRetryInterceptor(); add dpopRetryInterceptor() OkHttp interceptor that caches DPoP-Nonce and retries 401 once - TokenSource.java: wrap nonce String as new Nonce(nonce) to match DefaultDPoPProofFactory.createDPoPJWT signature; generalize RSAKey to JWK+JWSAlgorithm to support EC keys for ES256/ES384/ES512 - SDKBuilder.java: update to JWK+JWSAlgorithm, separate SRT key from DPoP key (EC DPoP key auto-generates RSA for SRT), wire dpopRetryInterceptor into OkHttpClient for KAS and all platform services; add dpopAlgorithm() builder method - Add TokenSourceTest and DPoPRetryInterceptorTest (6 new tests) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Add DPoP configuration flags to the tdf cmdline tool (shared by encrypt and decrypt via buildSDK()): - --dpop[=<alg>]: enable DPoP; optional algorithm (RS256, RS384, RS512, ES256, ES384, ES512); defaults to RS256; generates ephemeral key - --dpop-key <path>: use PEM-encoded private key from file; algorithm inferred from key type (EC or RSA); combinable with --dpop=<alg> Both flags work for encrypt and decrypt subcommands. Help text contains "dpop" so the grep probe matches: encrypt --help | grep -i dpop. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…e.INHERIT Add scope = CommandLine.ScopeType.INHERIT to --dpop and --dpop-key so they appear in `help encrypt` and `help decrypt` (not just the parent `tdf` help), allowing the tests-repo cli.sh probe to pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Gemini review: System.exit() in Supports.run() abruptly terminates the JVM and prevents unit testing. Switch to Callable<Integer> so picocli handles the exit code via CommandLine.execute(). Also remove `required = true` from --client-id, --client-secret, and --platform-endpoint so `supports dpop` can run without auth credentials (it performs a local capability check, not a platform call). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
… imports Adds EC key, origin-isolated nonce, and empty-nonce guard tests to TokenSourceTest; removes three nl.altindag.ssl imports from SDKBuilder that were added without a corresponding pom.xml dependency. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
f8baeab to
244f4ba
Compare
- Add test cases for Command.Supports subcommand - Implement support for dpop_nonce_challenge feature - Add JUnit 5 and AssertJ test dependencies to cmdline module
- Validate WWW-Authenticate before retry/cache: only honor 401s carrying scheme=DPoP and error=use_dpop_nonce (RFC 9449 §8). A bare DPoP-Nonce header from any 401 no longer triggers a retry or poisons the cache. - Cache rotated nonces after every chain.proceed(), not just on 401s (RFC 9449 §8.1) — the next request now picks up rotations from 200s without an extra round-trip. - Clear the ThreadLocal<URL> defensively on entry and on exception so a failure in getAuthHeaders() cannot leak the URL into the next call on the same worker thread. Tests cover: single-retry guarantee, three negative WWW-Authenticate shapes (no challenge, Basic, DPoP error=invalid_token), 200-response nonce rotation cached for the next request, and 10-thread concurrent retry smoke test using a stateful Dispatcher (FIFO queues can't deliver alternating 401/200 reliably under concurrent load).
- Validate DPoP JWK/JWS-algorithm compatibility at TokenSource construction (new DpopKeyValidation helper, also used by SDKBuilder for EC curve→algorithm inference): RSA keys require RS*/PS*, EC keys must match curve to ES* (P-256↔ES256, P-384↔ES384, P-521↔ES512), other JWK types are rejected. Mismatches now fail fast at build time instead of at first proof. - Replace the broad catch(Exception) in getToken() with specific catches: SDKException passes through unwrapped (no double-wrapping of use_dpop_nonce errors), IOException/JOSEException/ParseException/ MalformedURLException each get a distinct, endpoint-named message. - Reject a 200 response with no access_token as a defensive guard against a null token being cached and producing 'DPoP null' headers. - Include the token endpoint URI in the use_dpop_nonce-missing-header warn log and promote the unknown-token-type log from trace to warn, since both indicate AS protocol violations. - Cover EC→RSA SRT key separation and the inverse RSA-reuse case with SDKBuilderTest cases. The EC path was previously untested even though it's the load-bearing motivation for the JWK generalization. Tests cover: nonce origin keying by port/scheme/default-port, parse errors attributed to the token endpoint, three JWK/algorithm mismatch cases, and EC↔RSA SRT signer behavior.
The --client-id/--client-secret/--platform-endpoint flags were demoted from required=true so that 'tdf supports <feature>' (used by xtest harnesses to probe SDK capabilities) could run without credentials. The side effect was that 'tdf encrypt' and 'tdf decrypt' silently passed picocli validation and failed deep inside SDKBuilder.build() with an opaque error. Keep the picocli annotations optional and instead enforce in buildSDK() with picocli's ParameterException, which produces the standard 'Missing required option: ...' error + usage and exits with code 2. 'tdf supports' is unaffected since it does not call buildSDK().
Picocli's default execution handler prints only ex.getMessage(), which is null for many failure modes (NPE, etc.). Exceptions thrown during CommandLine construction or by picocli itself bypass that handler entirely and would otherwise terminate the JVM silently. Wrap main() in a top-level try/catch that calls printStackTrace() — the first line is the exception's toString (class + message) so the failure category is always identifiable, and the stack trace gives diagnostic depth for bug reports. Normal exit codes from picocli's execute() are unaffected.
X-Test Failure Report |
- AuthInterceptor: log exception at DEBUG level when DPoP retry chain.proceed() throws - Command: -v/--verbose now raises root log level to DEBUG (only if currently coarser) - log4j2.xml: default root level changed from trace to info
If Keycloak (or any AS) returns token_type=Bearer despite the SDK sending a DPoP proof, the prior behavior was to emit "Authorization: DPoP <bearer>" which misuses the scheme (RFC 9449 §7.1) and is rejected by any DPoP-enforcing resource server. TokenSource now remembers the scheme the AS declared (DPoP vs Bearer) and getAuthHeaders() emits a plain Bearer credential without a DPoP proof on downgrade. AuthInterceptor only sets the DPoP request header when a proof is present. A single WARN is logged on downgrade to flag the IdP misconfiguration.
RFC 9449 §4.2 requires the htu claim to be the request URI without
query and fragment, and Nimbus enforces it by throwing
IllegalArgumentException("The HTTP URI (htu) must not have a query").
When the OkHttp dpopRetryInterceptor handed Nimbus a URL whose query
string came from the caller (e.g. a KAS rewrap URL), proof creation
blew up inside the OkHttp Dispatcher thread and surfaced as
'error getting kas servers'.
Normalize every URI fed to DefaultDPoPProofFactory through a single
htuOf() helper that strips both query and fragment.
After 38ac01f the SDK correctly downgrades to the Bearer scheme when the token endpoint returns token_type=Bearer, so the shared sdkServicesSetup helper stopped sending a DPoP header and the three tests calling it (testCreatingSDKServicesPlainText, testPlatformPlainTextAndIDPWithSSL, testSDKServicesWithTruststore) failed at the 'expecting DPoP header' assertion. The test intent is the DPoP path, so make the mock token endpoint advertise DPoP.
X-Test Failure Report |
- §8.1 covers nonce *syntax*; the rotation/provision mechanic is §8.2.
- The OkHttp dpopRetryInterceptor handles resource-server traffic (KAS,
platform-services Connect client); that is RFC 9449 §9, not §8.
- Delete the obsolete docs/superpowers/{plans,specs}/2026-06-16-* files:
they describe a ~25-line, 3-file plan that no longer matches what
shipped (2200+ lines, 14 files, new public API surface).
…tform_issuer Previously SDKBuilder.getAuthInterceptor would silently warn-and-return-null when the well-known configuration omitted platform_issuer, which also disabled the dpopRetryInterceptor (gated on authInterceptor != null). A caller who explicitly opted into DPoP via dpopKey()/dpopAlgorithm() would then watch every request silently downgrade to no-auth and 401 from a DPoP-enforcing resource server with no client-side breadcrumb. Now the explicit-DPoP case throws SDKException with a message naming both DPoP and platform_issuer. The pre-existing no-auth fallback (no DPoP key configured) still warn-and-returns-null. The two SRT-derivation tests that relied on the silent fallback path are updated to mock a real OIDC endpoint, since 'dpopKey set + no token endpoint' is no longer a supported configuration.
Previously Command.applyDPoPOptions caught Exception and wrapped everything as 'Failed to configure DPoP: <message>', collapsing file-not-found, malformed PEM, unsupported algorithm, and key generation failures into one opaque RuntimeException. A public-key-only PEM was accepted without complaint and only failed deep inside proof generation. A --dpop-key with --dpop=<alg> mismatch was deferred to TokenSource construction. Refactor: - New CliDpopOptions static helper owns parse + validate + private-key check. Returns DpopMaterial (jwk + alg) or Optional.empty(); throws IllegalArgumentException with user-actionable messages. - applyDPoPOptions delegates to the helper and wraps IAE as CommandLine.ParameterException so picocli exits with USAGE (2) instead of a generic stack trace. - Promote DpopKeyValidation to public so cmdline can reuse the validation rules instead of duplicating them. Latent bug fixed: bcpkix-jdk18on was only test-scoped via the sdk module, so the production CLI's JWK.parseFromPEMEncodedObjects call for --dpop-key would have thrown NoClassDefFoundError. Adding it as a cmdline runtime dependency. Tests: - New CliDpopOptionsTest (16 cases): all six supported algorithms, default RS256, unsupported algorithm, missing/malformed/public-only PEM, RSA vs EC PEM acceptance and alg inference, RSA-key + ES256 mismatch. - Two new end-to-end CommandTest cases covering the unsupported-algorithm and missing-key-file ParameterException paths through CommandLine.execute.
Six DEBUG-level log sites tagged 'DPoP path=<stream|unary|unary-response|okhttp|okhttp-retry|okhttp-retry-response>' to triage the CI failure where the platform DPoP validator rejected proofs with htm=GET but expected POST. Each entry records the URL, the HTTP method that flows into the proof, the Authorization scheme (Bearer/DPoP — never the token), and a parsed DPoP claim summary (htm, htu, jti, nonce). Both the connect-layer method (request.httpMethod.name) and the outgoing OkHttp method (chain.request().method) are logged so a divergence between the two is visible. Helpers: - authScheme(): redacts the Authorization header to just its scheme. - dpopSummary(): parses the DPoP proof JWT and emits the claims that matter, falling back to <unparseable> on error. DEBUG level only — zero cost in production; surfaces via --verbose on the CLI.
Connect-RPC's GET extension rewrites idempotent POST RPCs to GET on the wire (with the request payload moved into the query string). The Connect interceptor stamps the DPoP proof before that rewrite happens, so htm=POST ends up on a GET request and the server rejects: 'incorrect htm claim in DPoP JWT; received [POST], but should match [[GET]]' This is the same class of mismatch that commit 16f16c9 fixed for htu (Connect-GET appends ?base64=&connect=v1&...&message=... to the URL, drifting htu the same way). htm wasn't covered. Disable Connect-GET on the authenticated ProtocolClient where DPoP proofs are attached. Keep it enabled on the unauthenticated bootstrap client (well-known config fetch) since no proof is sent there. Cost: authenticated unary RPCs (ListKeyAccessServers, etc.) round-trip as POST instead of GET. Those calls were never going to be CDN-cacheable anyway because each carries a per-request DPoP proof.
|



Summary
Adds comprehensive DPoP (RFC 9449) support to the Java SDK as part of the Keycloak v26 upgrade and DPoP implementation effort.
Related Jira: https://virtru.atlassian.net/browse/DSPX-3397
Test Scenario: xtest/scenarios/DSPX-3397.yaml (in tests repo)
Changes
Core DPoP Infrastructure
Map<String, String> nonceCache)getAuthHeaders()to accept optional nonce parameter for proof generationcacheNonce()method to store nonces from server responsesDPoP-Nonceheaders from successful (200) responsesConfiguration & Extensibility
dpopKey(RSAKey)method to allow caller-supplied DPoP keyssupports dpopCLI subcommand for xtest feature detection (exits 0)Implementation Notes
DefaultDPoPProofFactoryfor RFC 9449 compliant proof generationjti,htm,htu,iat(plusathfor resource endpoints)Feature Detection
The Java SDK now exposes DPoP support via:
tdf supports dpop # exits 0 if supported, 1 otherwiseThe xtest wrapper in the tests repo at
xtest/sdk/java/cli.shshould call this command to detect support.Testing
SDKBuilderTestandKASClientTestcontinue to passAuthorization: DPoP <token>+DPoPheader with proof)Related PRs
This PR is part of a multi-repo feature implementation. Coordinate with:
Checklist
supports dpop)